Vendor crate dependencies in sourcejobs - #1144
Conversation
There was a problem hiding this comment.
I'll add RPM changes to this PR as well.
It would be a good idea to leverage "Multiple Upstream Tarballs" to keep the vendor crates separate. The unpacking story is pretty clean.
We also need to make this process "opt-in", likely based on a specific configuration in the <export> section of the package.xml file.
|
|
||
|
|
||
| def build_sourcedeb(sources_dir, os_name=None, os_code_name=None): | ||
| def _vendor_cargo_crates(sources_dir): |
There was a problem hiding this comment.
This should be refactored into a common location so that the RPM builds can invoke it as well.
There was a problem hiding this comment.
Moved into cargo.py so that it can be re-used. Also modified the function so that it won't assume debian/vendor, and now instead receives a path, since it might be different for RPM.
| cmd = [ | ||
| 'cargo', 'vendor', '--locked', os.path.join('debian', 'vendor')] |
There was a problem hiding this comment.
As discussed, we won't be able to use --locked because it would require re-generation of the lock file every time an upstream ROS cargo package version is updated.
| cmd = [ | |
| 'cargo', 'vendor', '--locked', os.path.join('debian', 'vendor')] | |
| cmd = [ | |
| 'cargo', 'vendor', os.path.join('debian', 'vendor')] |
There was a problem hiding this comment.
Yes, I'm iterating on that locally. quilt complains if we generate a lockfile and ship it if it wasn't part of the original source, so I'm currently running cargo vendor and later delete the file. commit coming later
There was a problem hiding this comment.
I'm covering both possibilities in current implementation, not quite sure if the locked path will break, but can be easily removed after testing.
| ' --net=host' + | ||
| ' --net=$docker_network' + |
There was a problem hiding this comment.
I think we should keep existing precedent and still allow network access during these builds, but we should update the cargo configuration to disallow network access as part of the Bloom template.
https://doc.rust-lang.org/cargo/reference/config.html#netoffline
There was a problem hiding this comment.
Would you agree to disable it here for developing and then restore it? I wouldn't like us to hit a specific corner case of cargo ignoring its own --offline flag. (One case, apparently, build scripts and proc macros ignore this flag, as per LLM discussion)
There was a problem hiding this comment.
Just curious, any reason to keep existing precedent? I thought it was a bug when I found it, and imagined it was a good opportunity to do it differently here
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
94ab9b1 to
2fc4fc2
Compare
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 participants
We ship the source code of the dependencies required for a crate using cargo vendor before building it.
We use a per-package flag to enable it / disable it.
Decision: we leave git sources and private registries out of the vendoring, at least for now